Skip to content

Use testsuite for SVD tests #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
kshyatt merged 11 commits into from
Jan 6, 2026
Merged

Use testsuite for SVD tests #137

kshyatt merged 11 commits into from
Jan 6, 2026

Conversation

@kshyatt
Copy link
Member

@kshyatt kshyatt commented Jan 5, 2026

No description provided.

@kshyatt kshyatt requested review from Jutho and lkdvos January 5, 2026 10:12
@kshyatt kshyatt force-pushed the testsuite-svd branch 3 times, most recently from bee135b to 23300d1 Compare January 5, 2026 13:41
@codecov
Copy link

codecov bot commented Jan 5, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/implementations/svd.jl 80.24% <100.00%> (-12.43%) ⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Jutho
Copy link
Member

Jutho commented Jan 5, 2026

Still some failing tests for eig_trunc!. It seems to be a tolerance issue, since the failing comparisons are actually not too bad:

Expression: diagview(D4) ≈ (diagview(D))[1:2]
   Evaluated: ComplexF32[0.89972997f0 + 0.0f0im, 0.30034363f0 + 0.0f0im] ≈ Float32[0.9, 0.3]

and

  Expression: ≈(ϵ3, norm((diagview(D))[3:4]), atol = atol)
   Evaluated: 0.10009178f0 ≈ 0.10049876f0 (atol=0.00034526698)

The first one doesn't seem to have a tolerance specified, so maybe that is fixed by adding the current default. The second one seems fine if the tolerance were just a little bit bigger, so maybe a factor 2 extra would be good.

@kshyatt
Copy link
Member Author

kshyatt commented Jan 5, 2026

It's odd that it only happened on Windows... I'll look further

@Jutho
Copy link
Member

Jutho commented Jan 5, 2026

That's a random number effect I think. Though it is often on Windows it seems. I guess that OS has some special affinity for bugs 😄 .

lkdvos
lkdvos reviewed Jan 5, 2026
View reviewed changes
test/testsuite/qr.jl
Copy link
Member

@lkdvos lkdvos Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not also be changed for the lq tests?

Copy link
Member Author

@kshyatt kshyatt Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was already fixed there?

Jutho
Jutho reviewed Jan 5, 2026
View reviewed changes
Jutho
Jutho reviewed Jan 5, 2026
View reviewed changes
Jutho
Jutho reviewed Jan 5, 2026
View reviewed changes
Jutho
Jutho reviewed Jan 5, 2026
View reviewed changes
@kshyatt kshyatt enabled auto-merge (squash) January 6, 2026 14:59
@kshyatt kshyatt merged commit 3326e00 into main Jan 6, 2026
10 checks passed
@kshyatt kshyatt deleted the testsuite-svd branch January 6, 2026 15:00
lkdvos referenced this pull request Jan 8, 2026
@kshyatt @lkdvos
Bump v0.6.2 (#149)
91a8a69 
* Bump version

* update changelog

---------

Co-authored-by: Lukas Devos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lkdvos lkdvos lkdvos left review comments

@Jutho Jutho Jutho approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kshyatt @Jutho @lkdvos